tweak(gui): Decouple GUI transition and world animation timing from render update by bobtista · Pull Request #2056 · TheSuperHackers/GeneralsGameCode

bobtista · 2026-01-04T05:58:28Z

Summary

GUI window transitions now advance at a consistent rate regardless of frame rate
2D world animation icons (healing, promotion, crate collection effects) rise at consistent speed regardless of frame rate

Changes

TransitionGroup::m_currentFrame changed from Int to Real to support fractional frame accumulation
TransitionGroup::update() scales frame advancement by TheFramePacer->getActualLogicTimeScaleOverFpsRatio()
InGameUI world animation Z-rise calculation now scales by the same time factor

Test plan

Verify GUI transitions (menu fades, button flashes) play at the same speed at 30fps and higher frame rates
Verify 2D world icons (healing icons, veteran stars, crate pickups) rise at consistent speed regardless of frame rate

greptile-apps · 2026-02-03T22:25:32Z

Greptile Summary

This PR decouples GUI window transition timing and 2D world animation Z-rise from the raw render frame rate by introducing a Real fractional frame accumulator in TransitionGroup and a per-frame time-scale multiplier in InGameUI::updateAndDrawWorldAnimations. The transition fix is well-implemented, but the world animation fix uses the wrong FramePacer method.

TransitionGroup::update() now advances m_currentFrame by getBaseOverUpdateFpsRatio() per render frame, correctly stepping through every discrete transition state even when the render rate dips below the 30 fps baseline.
InGameUI::updateAndDrawWorldAnimations() multiplies the Z-rise by getActualLogicTimeScaleOverFpsRatio(), which returns 1.0 in the default uncapped single-player mode (because getActualLogicTimeScaleFps() returns UncappedFpsValue = 1,000,000), leaving the animation speed frame-rate dependent unless the logic-time-scale feature is explicitly enabled. The same issue exists in the GeneralsMD variant.

Confidence Score: 4/5

The transition timing fix is correct and safe to merge. The world animation Z-rise fix does not achieve frame-rate independence in the default single-player configuration.

The GUI transition path is solid: the Real accumulator with getBaseOverUpdateFpsRatio() correctly normalizes advancement across render rates. The InGameUI Z-rise fix is the problem: getActualLogicTimeScaleOverFpsRatio() returns 1.0 whenever the logic-time-scale feature is off (the default), so the multiplied scale factor is always 1.0 and the animation rises at the same frame-rate-dependent speed as before. Both Generals and GeneralsMD variants share this defect.

Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp and GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp — the zRiseTimeScale calculation needs to use getBaseOverUpdateFpsRatio() to match the approach taken in the transition code.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameClient/GameWindowTransitions.h	Changes `m_currentFrame` from `Int` to `Real` with an updated doc-comment; correct and self-contained.
Core/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp	Correctly uses `getBaseOverUpdateFpsRatio()` to advance the fractional frame accumulator; step-loop with early-break for finished transitions is sound.
Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp	Uses `getActualLogicTimeScaleOverFpsRatio()` which returns 1.0 in the default uncapped mode, making the Z-rise fix ineffective unless the logic-time-scale feature is explicitly enabled.
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp	Same `getActualLogicTimeScaleOverFpsRatio()` issue as in the Generals variant — Z-rise remains frame-rate dependent under the default game configuration.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Render Frame] --> B[TransitionGroup::update]
    A --> C[InGameUI::updateAndDrawWorldAnimations]

    B --> D["timeScale = getBaseOverUpdateFpsRatio()\n= BaseFps / renderFps"]
    D --> E["m_currentFrame += dirMult * timeScale"]
    E --> F{newFrame != prevFrame?}
    F -- Yes --> G[Step through each integer frame\nand call tWin->update]
    F -- No --> H[Return early, no state change]
    G --> I{All transitions finished?}
    I -- Yes --> J[Break early]
    I -- No --> G

    C --> K["zRiseTimeScale = getActualLogicTimeScaleOverFpsRatio()\n= min(1.0, logicFps / renderFps)"]
    K --> L{Logic time scale enabled?}
    L -- Yes --> M["Returns logicFps/renderFps ✓\ne.g. 30/60 = 0.5"]
    L -- No/Default --> N["logicFps = UncappedFpsValue (1,000,000)\nReturns 1.0 regardless of renderFps ✗"]
    M --> O["z += zRisePerSecond / 30 * 0.5\n= frame-rate independent ✓"]
    N --> P["z += zRisePerSecond / 30 * 1.0\n= still frame-rate dependent ✗"]

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Render Frame] --> B[TransitionGroup::update]
    A --> C[InGameUI::updateAndDrawWorldAnimations]

    B --> D["timeScale = getBaseOverUpdateFpsRatio()\n= BaseFps / renderFps"]
    D --> E["m_currentFrame += dirMult * timeScale"]
    E --> F{newFrame != prevFrame?}
    F -- Yes --> G[Step through each integer frame\nand call tWin->update]
    F -- No --> H[Return early, no state change]
    G --> I{All transitions finished?}
    I -- Yes --> J[Break early]
    I -- No --> G

    C --> K["zRiseTimeScale = getActualLogicTimeScaleOverFpsRatio()\n= min(1.0, logicFps / renderFps)"]
    K --> L{Logic time scale enabled?}
    L -- Yes --> M["Returns logicFps/renderFps ✓\ne.g. 30/60 = 0.5"]
    L -- No/Default --> N["logicFps = UncappedFpsValue (1,000,000)\nReturns 1.0 regardless of renderFps ✗"]
    M --> O["z += zRisePerSecond / 30 * 0.5\n= frame-rate independent ✓"]
    N --> P["z += zRisePerSecond / 30 * 1.0\n= still frame-rate dependent ✗"]

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp:5508
**Wrong FramePacer method — fix is a no-op in the default game mode**

`getActualLogicTimeScaleOverFpsRatio()` returns `min(1.0f, logicFps / renderFps)`. When logic time scale is *disabled* (the default), `getActualLogicTimeScaleFps()` returns `RenderFpsPreset::UncappedFpsValue` (= 1 000 000), so this ratio collapses to `min(1.0, 1000000/renderFps) = 1.0` at any practical frame rate. `zRiseTimeScale` is thus always 1.0 in single-player without the logic-time-scale feature enabled, and the Z-rise remains frame-rate dependent.

The transition code correctly uses `getBaseOverUpdateFpsRatio()` (= `BaseFps / renderFps` = `30 / renderFps`), which scales down proportionally at any render fps without requiring the logic time scale feature. The world-animation fix should use the same method. The same issue is present in `GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp`.

_{Reviews (6): Last reviewed commit: "fix(gui): Check transition completion du..." | Re-trigger Greptile}

greptile-apps

_{6 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-03T22:25:41Z

Additional Comments (1)

Generals/Code/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp
[P2] This file still uses NULL (TheTransitionHandler = NULL;). New code in this PR also adds FramePacer usage; consider switching to nullptr for null pointer literals to match the repo’s C++ style.

GameWindowTransitionsHandler *TheTransitionHandler = nullptr;

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp
Line: 62:62

Comment:
[P2] This file still uses `NULL` (`TheTransitionHandler = NULL;`). New code in this PR also adds `FramePacer` usage; consider switching to `nullptr` for null pointer literals to match the repo’s C++ style.

```suggestion
GameWindowTransitionsHandler *TheTransitionHandler = nullptr;
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Caball009 · 2026-02-23T03:29:32Z

I frequently find myself increasing the render frame rate at the menu because I find the window transitions too slow at 30 fps, and would probably like it better if they played close to ~1.75x the original speed. Is there anything this PR can do in that regard?

ElTioRata · 2026-02-23T04:43:57Z

I agree with Caball. It would be nice to have a setting to adjust GUI speed (x1.0... x1.25... x1.5x... x1.75... x2.00... and so on).

xezon · 2026-02-23T07:42:02Z

Feature for scaling animation speed is unrelated to decoupling step and better be follow up change.

githubawn · 2026-02-23T15:39:28Z

Looks good.
Maybe fully skip animations under -quickstart in the follow-up, cleaner than adding explicit speed controls.

…all game states

Caball009 · 2026-02-28T18:44:32Z

There are still a couple of to-dos in the PR description.

Caball009

Looks good to me.

I'm still in favor of making the float assignments explicit for m_currentFrame, but it makes no functional difference.

xezon

I tested this and something appears to be broken.

Sometimes the UI just gets stuck. I was testing with long SSD load times because Visual Studio was parsing which bogs down the SSD.

After clicking Options menu

After clicking Skirmish Menu

After clicking Credits Menu

Caball009 · 2026-03-26T19:02:40Z

Sometimes the UI just gets stuck.

Is this something you can reproduce? I don't think I've seen anything like this during my testing.

xezon · 2026-03-26T19:05:42Z

I ran into this several times when testing. My SSD was slow, but still it was unusual behavior. I can retest if this is not reproducible for others.

Caball009 · 2026-03-28T14:50:06Z

I can retest if this is not reproducible for others.

Please do.

Caball009 · 2026-04-10T14:25:45Z

I ran into this several times when testing. My SSD was slow, but still it was unusual behavior. I can retest if this is not reproducible for others.

Can you take a look at this? I cannot reproduce it, and this PR probably shouldn't be blocked from merging if you can't reproduce it either

githubawn · 2026-04-10T16:04:18Z

I replicated Xezon's bug by running this on a networked drive.
This would also probably break on SD installs.

The logic seems to fail on long IO stalls.

githubawn · 2026-04-10T16:34:12Z

Added a single sleep to replicate this, the menu won't even launch on this. But it does on main.

GameEngine.cpp

void GameEngine::execute()
{
#if defined(RTS_DEBUG)
	DWORD startTime = timeGetTime() / 1000;
#endif

	// pretty basic for now
	while( !m_quitting )
	{
		::Sleep(100);

30 will randomly bug out the menu after a few switches.

Implementation needs changes.

Caball009 · 2026-04-12T17:32:04Z

Added a single sleep to replicate this, the menu won't even launch on this. But it does on main.

~~I just tried with the main branch + sleep and the menu doesn't load, so it doesn't seem like this PR is at fault.~~
Edit: I can confirm that main + sleep still works, but PR + sleep does not.

~~I suspect that the window transition code is too brittle to handle the sleep as it probably skips over hard-coded transitions.~~

xezon · 2026-04-18T13:46:20Z

Yes the bug still happens. Got stuck here:

 	generalsv.exe!TransitionGroup::isFinished() Line 293	C++
>	generalsv.exe!GameLogic::startNewGame(bool saveGame) Line 1895	C++
 	generalsv.exe!GameLogic::update() Line 3108	C++
 	generalsv.exe!SubsystemInterface::UPDATE() Line 74	C++
 	generalsv.exe!GameEngine::update() Line 749	C++
 	generalsv.exe!Win32GameEngine::update() Line 94	C++
 	generalsv.exe!GameEngine::execute() Line 804	C++
 	generalsv.exe!GameMain() Line 59	C++
 	generalsv.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 874	C++
 	[Inline Frame] generalsv.exe!invoke_main() Line 102	C++
 	generalsv.exe!__scrt_common_main_seh() Line 288	C++
 	kernel32.dll!76385d49()	Unknown
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	
 	ntdll.dll!7767d83b()	Unknown
 	ntdll.dll!7767d7c1()	Unknown

I suspect it has something to do with FlashTransition::update where if the frame does not perfectly hit the expected value, the transition never finishes. This certainly is possible when the frame rate is not stable.

Caball009 · 2026-05-04T10:09:04Z

@bobtista Can you take a look at the stuck menu issue?

xezon · 2026-05-05T18:46:55Z

+			it++;
+		}
+
+		if( isFinished() )


This goes through the list a second time. This can probably be optimized by returning finished result in update function above and then use that to determine finished status.

Or just check isFinished for each transition group after each update.

bobtista · 2026-06-19T19:07:18Z

I've addressed the remaining review comments and tested the menu-stall repro locally:

Tested generalsv.exe with the reviewer repro hook, ::Sleep(100), in GameEngine::execute().
Clicked through the shell map menu buttons in both Generals and Zero Hour
Ready for re-review as needed.

greptile-apps Bot reviewed Feb 3, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp Outdated

xezon added this to the Decouple logic and rendering milestone Feb 5, 2026

xezon added GUI For graphical user interface Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Feb 23, 2026

bobtista added 3 commits February 24, 2026 11:55

Decouple GUI transition and world animation timing from render update

95357c3

Replicate to generals

cff9563

Use getBaseOverUpdateFpsRatio() for frame-rate independent timing in …

e6cd525

…all game states

bobtista force-pushed the bobtista/fix-gui-animation-timing branch from 276ed5c to e6cd525 Compare February 24, 2026 16:59

Caball009 reviewed Feb 25, 2026

View reviewed changes

Comment thread Core/GameEngine/Include/GameClient/GameWindowTransitions.h

Caball009 previously approved these changes Mar 24, 2026

View reviewed changes

style: Use explicit float assignments for m_currentFrame

901bd6c

xezon approved these changes Mar 24, 2026

View reviewed changes

xezon requested changes Mar 24, 2026

View reviewed changes

xezon reviewed Apr 18, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp Outdated

bobtista added 2 commits April 18, 2026 12:33

fix(gui): Use logic time scale for world animation Z-rise

5e51f64

fix(gui): Replicate to Generals

e5c8d45

fix(gui): Step every integer transition frame to prevent stuck menu

c714914

xezon reviewed May 5, 2026

View reviewed changes

fix(gui): Check transition completion during stepped updates

b4e9b45

Conversation

bobtista commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

Uh oh!

greptile-apps Bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps Bot commented Feb 3, 2026

Uh oh!

Caball009 commented Feb 23, 2026

Uh oh!

ElTioRata commented Feb 23, 2026

Uh oh!

xezon commented Feb 23, 2026

Uh oh!

githubawn commented Feb 23, 2026

Uh oh!

Uh oh!

Caball009 commented Feb 28, 2026

Uh oh!

Caball009 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

After clicking Options menu

After clicking Skirmish Menu

After clicking Credits Menu

Uh oh!

Caball009 commented Mar 26, 2026

Uh oh!

xezon commented Mar 26, 2026

Uh oh!

Caball009 commented Mar 28, 2026

Uh oh!

Caball009 commented Apr 10, 2026

Uh oh!

githubawn commented Apr 10, 2026

Uh oh!

githubawn commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Caball009 commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon commented Apr 18, 2026

Uh oh!

Uh oh!

Caball009 commented May 4, 2026

Uh oh!

xezon May 5, 2026

Choose a reason for hiding this comment

Uh oh!

xezon May 5, 2026

Choose a reason for hiding this comment

Uh oh!

bobtista commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bobtista commented Jan 4, 2026 •

edited

Loading

greptile-apps Bot commented Feb 3, 2026 •

edited

Loading

Caball009 left a comment •

edited

Loading

githubawn commented Apr 10, 2026 •

edited

Loading

Caball009 commented Apr 12, 2026 •

edited

Loading